fix(cli): add local_development.redis_enabled manifest opt-out (MAY-1086)#365
fix(cli): add local_development.redis_enabled manifest opt-out (MAY-1086)#365IdkwhatImD0ing wants to merge 4 commits into
Conversation
…086) agentex agents run unconditionally sets REDIS_URL=redis://localhost:6379 in the agent process env. Combined with the module-level RedisStreamRepository instantiated by `from agentex.lib import adk`, this causes silent request hangs for agents that don't use adk.messages/adk.streaming when no local Redis is reachable (e.g. Temporal-direct async agents in restricted dev pods). The lazy redis.asyncio client only fails on first publish, so there is no startup error to point at the misconfiguration. Add an explicit `local_development.redis_enabled: bool = true` flag. Default true preserves existing behavior; users with no local Redis can set false to skip the REDIS_URL default. acp_type is not a reliable discriminator here — the codebase has acp_type=async tutorials that legitimately call adk.messages.create. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address staff-engineer review on #365: - Pop REDIS_URL from the inherited env when redis_enabled is false. The previous implementation only avoided seeding a default, so a stale REDIS_URL exported by the parent shell still leaked into the agent process and reproduced the same silent hang the opt-out was meant to prevent. - Drop the unreachable `local_development is None` short-circuit; the dict literal above already dereferences manifest.local_development with a # type: ignore[union-attr], so the None branch can never run. - Add tests/lib/cli/test_run_handlers.py covering the three branches: default seeds REDIS_URL, opt-out with clean parent env clears it, opt-out with a parent-shell REDIS_URL still clears it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Follow-up commit
Deferring the other review points:
|
Use a tmp_path YAML fixture rendered from a minimal inline template instead of reading examples/tutorials/.../manifest.yaml, so the test survives tutorial renames or removals while still exercising the real AgentManifest.from_yaml loader. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Two prior reports of this exact silent-hang behavior in #sgp-agentex predate this fix, both consistent with the root cause documented in the PR description:
So this is the third real-world instance of the same pattern. The opt-out will fix the crash. |
Summary
agentex agents rununconditionally setsREDIS_URL=redis://localhost:6379in the agent process env. Combined with the module-levelRedisStreamRepositoryfromfrom agentex.lib import adk, agents that don't actually useadk.messages/adk.streamingsee silent request hangs when no local Redis is reachable (e.g. Temporal-direct async agents running in restricted dev pods). The lazyredis.asyncioclient only fails on first publish, so there's no startup error to surface.local_development.redis_enabled: bool = true(default) flag on the agent manifest. Default preserves existing behavior; users with no local Redis can setfalseto opt out of theREDIS_URLdefault.acp_typeis not a reliable discriminator:examples/tutorials/10_async/00_base/110_pydantic_aiand100_langgraphareacp_type: asyncand legitimately calladk.messages.create(...); severalacp_type: synctutorials also useadk.messages. Hence the explicit per-manifest opt-out instead of anacp_type-based rule.Files
src/agentex/lib/sdk/config/local_development_config.py— newredis_enabled: bool = Truefield onLocalDevelopmentConfig.src/agentex/lib/cli/handlers/run_handlers.py—create_agent_environmentskips theREDIS_URLdefault whenlocal_development.redis_enabledisfalse.Example manifest opt-out
Test plan
rye run pyrighton modified files — 0 errors.rye run ruff checkon modified files — clean.REDIS_URLset;redis_enabled: false→REDIS_URLabsent (verified againstexamples/tutorials/10_async/00_base/110_pydantic_ai/manifest.yaml).tests/...matchinglocal_development|run_handlers|manifest) passes.Linear
MAY-1086
🤖 Generated with Claude Code
Greptile Summary
This PR adds a
local_development.redis_enabled: bool = Truefield toLocalDevelopmentConfigand gates the automaticREDIS_URL=redis://localhost:6379injection increate_agent_environmenton that flag. The fix addresses silent request hangs in dev pods where no local Redis is reachable.local_development_config.py: Addsredis_enabledfield withdefault=Trueto preserve existing behavior; agents that don't useadk.messages/adk.streamingcan set it tofalse.run_handlers.py: The conditional now setsREDIS_URLinenv_varswhen opted in, and actively pops any inherited parent-shellREDIS_URLwhen opted out — ensuring the opt-out is clean even when the shell exports the variable.test_run_handlers.py: Three parametric scenarios cover the default, opt-out-clean, and opt-out-with-stale-parent-env cases.Confidence Score: 5/5
Safe to merge; the change is an additive opt-out flag with a backward-compatible default, and the core env-mutation logic is correctly ordered relative to the final env.update call.
The new field defaults to True, so all existing manifests see no behavior change. The opt-out path pops REDIS_URL from the os.environ snapshot before env.update(env_vars) is called, so the removal is durable. Tests cover the three relevant scenarios. No other code paths are affected.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[create_agent_environment] --> B["env = dict(os.environ)"] B --> C["Build env_vars: TEMPORAL_ADDRESS, AGENT_NAME, ACP_TYPE, ACP_URL, ACP_PORT"] C --> D{"manifest.local_development.redis_enabled?"} D -- "True (default)" --> E["env_vars['REDIS_URL'] = 'redis://localhost:6379'"] D -- "False" --> F["env.pop('REDIS_URL', None)"] E --> G["Apply agent_config.env overrides to env_vars"] F --> G G --> H["env.update(env_vars)"] H --> I["return env"]Reviews (3): Last reviewed commit: "test(cli): decouple run_handlers tests f..." | Re-trigger Greptile